Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improvements to vertex/silicon wrapper #396

Merged
merged 3 commits into from
Sep 25, 2024
Merged

Improvements to vertex/silicon wrapper #396

merged 3 commits into from
Sep 25, 2024
+61 −60

Conversation

armin-ilg
Copy link
Contributor

@armin-ilg armin-ilg commented Sep 24, 2024 •
edited
Loading

BEGINRELEASENOTES

  • No more warnings in silicon wrapper
  • Improvements in vertex builder printouts
  • Adding all materials of beam pipe also to material_plots_2D.py, as without having the beam pipe enabled, the vertex material budget estimation will fail.
  • Changed paths to .stl files in vertex to use https://fccsw.web.cern.ch/fccsw/filesForSimDigiReco/IDEA/IDEA_o1_v03/STL_FILES/ (still commented out due to overlaps)
    ENDRELEASENOTES

@BrieucF: As you requested I fixed the warnings in the silicon wrapper.

P.s:
I don't know how to squash all the commits, sorry that this is such a mess

@andresailer
Copy link
Contributor

Can you stop touching your branch for 5 minutes?

@armin-ilg
Removed repeated definition in silicon wrapper XML, improved printout…
b76f267 
… in vertex detector constructors, changed paths of .stl filesin vertex (still commented out due to overlaps)
@armin-ilg
Adding all beam pipe materials to both material scan scripts.
abe5d27
@armin-ilg
Improving material_plots(_2D).py scripts to take list ofmaterial stri…
c370259 
…ngs as input to be ignored and list of substrings in materials to be removed
@andresailer andresailer mentioned this pull request Sep 25, 2024
Open
@BrieucF
Copy link
Contributor

BrieucF commented Sep 25, 2024

Shall we merge this before or after #392?

@tmadlener
Copy link
Contributor

There will most likely be a conflict either way. So, I don't have any strong preferences.

@andresailer
Copy link
Contributor

Merging this, applying a styler and then cherry-picking the diff from #392 is probably easier than the other way around, and these changes here are only feature changes, so would not be nice to punish Armin :)

@BrieucF BrieucF merged commit 0b59886 into key4hep:main Sep 25, 2024
6 checks passed
@tmadlener tmadlener mentioned this pull request Sep 25, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andresailer andresailer andresailer left review comments

@BrieucF BrieucF BrieucF left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@armin-ilg @andresailer @BrieucF @tmadlener